Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace calls to dbg! with log::debug! + env_logger #89

Closed
wants to merge 2 commits into from

Conversation

jfarmer
Copy link
Contributor

@jfarmer jfarmer commented Nov 18, 2024

We can use the RUST_LOG environment variable to control the log level. According to the dbg documentation:

Note that the macro is intended as a debugging tool and therefore you should avoid having uses of it in version control for long periods (other than in tests and similar). Debug output from production code is better done with other facilities such as the debug! macro from the log crate.

I used log::error! in a few places where it seemed appropriate. Some of the dbg! calls probably make more sense as trace! or info!, but those can be changed later.

We can use the RUST_LOG environment variable to control the
log level.
@KonaeAkira KonaeAkira self-requested a review November 18, 2024 21:14
Copy link
Owner

@KonaeAkira KonaeAkira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! However, I would leave the debug prints in the tests as they are now for the following reasons:

  • Test output is not seen by the end user
  • When running tests using cargo test, the stderr output is only displayed in case the test fails.
  • The stuff that is currently printed is crucial for debugging failing test cases, so they should always be visible when a test fails.

@jfarmer
Copy link
Contributor Author

jfarmer commented Nov 18, 2024

Nice! However, I would leave the debug prints in the tests as they are now for the following reasons:

  • Test output is not seen by the end user
  • When running tests using cargo test, the stderr output is only displayed in case the test fails.
  • The stuff that is currently printed is crucial for debugging failing test cases, so they should always be visible when a test fails.

Roger.

@KonaeAkira KonaeAkira changed the title Replace calls to dbg! with log::debug! + env_logger Replace calls to dbg! with log::debug! + env_logger Nov 18, 2024
@KonaeAkira
Copy link
Owner

Closed, because debug output is a must-have for macro_solver_example.rs, and there is no way to add env_logger as a dependency for the example without also adding it for the entire solvers package.

@KonaeAkira KonaeAkira closed this Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants